Skip to content

feat: implement r+, r= and r- #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 30, 2024
Merged

Conversation

vohoanglong0107
Copy link
Contributor

No description provided.

Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Left a couple of nits, but otherwise looks very clean.

tester.get_comment().await?,
format!(
"Commit pr-{}-sha has been approved by `{}`",
default_pr_number(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's find to hardcode these values here to make the test simpler. If we ever change them in the future, the test will just fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah the reason I'm not hardcoding the values in was I didn't know where those values come from, and this was my attempt to explain the origin of those values for future me 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a bad reason! I hope we could make the default values so obvious that this wouldn't be needed though, and it would be immediately obvious from the test that 1 or default-user etc. is a special value :)

Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :)

@Kobzol Kobzol added this pull request to the merge queue Jun 30, 2024
Merged via the queue into rust-lang:main with commit dd3f8a3 Jun 30, 2024
2 checks passed
@vohoanglong0107 vohoanglong0107 deleted the feat-accept-pr branch June 30, 2024 13:13
@Kobzol Kobzol mentioned this pull request Dec 10, 2024
@Kobzol Kobzol mentioned this pull request Mar 11, 2025
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants